tree: add asset-aware materialization context - #1176
Conversation
Asset-bearing tree nodes use distinct taproot tweaks and carry units independently from carrier satoshis. Store this data in an outpoint-keyed context so extracted paths sign deterministically without changing Bitcoin-only trees. Keep per-node tweak selection inside Tree signing and retain the existing public signer API. Copy MuSig2 participant slices before aggregation because the local signer may sort caller-owned state.
3f0375b to
75de4e8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75de4e829a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Hey! Great work, currently reviewing this (side by side with #1177 ) |
|
|
||
| taprootTweak := sweepTapscriptRoot | ||
| if tweakLookup != nil { | ||
| taprootTweak = tweakLookup(node) |
There was a problem hiding this comment.
Signers trust operator-supplied tweaks; presence is checked, correctness is not.
The tweak lookup fails closed on a missing tweak, but a wrong tweak is signed without complaint. Since the tweak encodes the asset commitment, any signer using this path is trusting whoever populated the context for the exact value that determines the asset distribution. Please document on newSignerSession/NewTreeSignerSession that this hook is operator-trusted until client-side verification lands, and link the follow-up where signers recompute the expected tweak from the sealed package before signing.
| } | ||
|
|
||
| // NodeAssetAmount returns a node's subtree asset amount. | ||
| func (c *AssetTreeContext) NodeAssetAmount(node *Node) uint64 { |
There was a problem hiding this comment.
Amount lookups silently return zero if the re-indexing choreography is missed.
The dual keying (node pointer during structure building, outpoint after materialization) only works because the materializer in #1177 re-indexes every node once inputs are assigned. Any other clone/rehydrate/serialize path that misses this silently gets zero from NodeAssetAmount. Consider keying everything by outpoint and returning an error (not 0) before materialization, or moving the amount onto Node. At minimum, document the re-indexing requirement on SetNodeAssetAmount.
| BatchOutpoint: t.BatchOutpoint, | ||
| BatchOutput: t.BatchOutput, | ||
| SweepTapscriptRoot: t.SweepTapscriptRoot, | ||
| AssetContext: t.AssetContext, |
There was a problem hiding this comment.
Extracted paths carry the whole batch's asset data.
Both extractors prune the node graph but return the same AssetContext pointer, which holds amounts and sealed packages for every input in the batch. Latent today since the context has no wire format, but once it is serialized for client delivery, every client can enumerate the whole batch's asset distribution. Please project the context down to the extracted path's outpoints at extraction time, before a transport format cements this shape.
| } | ||
|
|
||
| // TestAssetTreeContextAmountsByIdentity tests node identity keying. | ||
| func TestAssetTreeContextAmountsByIdentity(t *testing.T) { |
There was a problem hiding this comment.
The pointer-to-outpoint fallback that makes extracted paths work is untested.
No test exercises ExtractPath* followed by NodeAssetAmount on a cloned node, which is exactly the fallback invariant from the comment on NodeAssetAmount. Please add a regression test.
Summary
Runtime isolation
This branch adds no asset runtime entry point. Bitcoin-only leaves keep
AssetAmountat zero,BuildStructurereturns a nilAssetContext, and public signer calls continue to use the tree-wide sweep tweak.